Attempt to estimate world in teleport when it's not specified. #6611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Moderocky
added
bug
sovdeeth
approved these changes
May 1, 2024
Comment on lines
132
to
141
| if (unknownWorld) { // we can't fetch the chunk without a world | ||
| for (Entity entity : entityArray) { | ||
| if (entity == null) | ||
| continue; | ||
| // assume it's a local teleport, use the first entity we find as a reference | ||
| loc = new Location(entity.getWorld(), loc.getX(), loc.getY(), loc.getZ(), loc.getYaw(), loc.getPitch()); | ||
| break find_a_world; | ||
| } | ||
| return next; // no entities = no chunk = nobody teleporting | ||
| } |
There was a problem hiding this comment.
I'm not 100% convinced this is great, but it seems like a very good assumption most of the time so whatever
APickledWalrus
requested changes
May 1, 2024
| if (entity == null) | ||
| continue; | ||
| // assume it's a local teleport, use the first entity we find as a reference | ||
| loc = new Location(entity.getWorld(), loc.getX(), loc.getY(), loc.getZ(), loc.getYaw(), loc.getPitch()); |
There was a problem hiding this comment.
Suggested change
| loc = new Location(entity.getWorld(), loc.getX(), loc.getY(), loc.getZ(), loc.getYaw(), loc.getPitch()); | |
| loc = loc.clone(); | |
| loc.setWorld(entity.getWorld()); |
Co-authored-by: Patrick Miller <[email protected]>
Moderocky
added
2.9
APickledWalrus
approved these changes
Jul 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I previously tried to fix this issue in the Entity utils teleport method, by estimating their target world from their source world if it wasn't specified.
Unfortunately,
EffTeleportdoes a lot of stuff that isn't teleporting (like setting the move event target, loading the chunk), so the last patch didn't fix that.This time I do my best to estimate a location in all cases if I can, or just return if I can't.
It's not ideal, but it's the best we can do to avoid an error while not just failing silently with no feedback for the user about why their teleport did nothing.
Are there any more of these?
Probably, but the only way to find them would be to check every single usage of
Locationacross all 500 and so files, and it's very hit-or-miss whether Bukkit requires a location or not.Target Minecraft Versions: any
Requirements: none
Related Issues: fixes #6610